refactor(rest-api-client): align client with rest-api-protocol and add typed ApiClient#1968
Merged
josecelano merged 8 commits intoJun 30, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the torrust-tracker-rest-api-client package to align with rest-api-protocol by introducing a higher-level, typed ApiClient while renaming the existing low-level transport client from Client to ApiHttpClient.
Changes:
- Renamed
Client→ApiHttpClientacross tests and E2E helpers. - Added a new typed
ApiClientplusClientError, and re-exported clients fromv1. - Added
torrust-tracker-rest-api-protocolas a dependency (and updatedCargo.lock).
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/servers/api/contract/stats/mod.rs | Updates imports to use ApiHttpClient after rename. |
| src/console/ci/qbittorrent_e2e/tracker/client.rs | Updates E2E wrapper to use ApiHttpClient and adjusts docs. |
| packages/rest-api-client/src/v1/mod.rs | Re-exports ApiClient and ApiHttpClient from v1. |
| packages/rest-api-client/src/v1/client.rs | Introduces ApiClient + ClientError, renames transport client to ApiHttpClient, and re-exports protocol AddKeyForm. |
| packages/rest-api-client/Cargo.toml | Adds torrust-tracker-rest-api-protocol dependency. |
| packages/axum-rest-api-server/tests/server/v1/contract/context/whitelist.rs | Updates contract tests to use ApiHttpClient. |
| packages/axum-rest-api-server/tests/server/v1/contract/context/torrent.rs | Updates contract tests to use ApiHttpClient. |
| packages/axum-rest-api-server/tests/server/v1/contract/context/stats.rs | Updates contract tests to use ApiHttpClient. |
| packages/axum-rest-api-server/tests/server/v1/contract/context/auth_key.rs | Updates contract tests to use ApiHttpClient and protocol AddKeyForm fields. |
| packages/axum-rest-api-server/tests/server/v1/contract/authentication.rs | Updates authentication contract tests to use ApiHttpClient. |
| docs/issues/open/1944-1938-si-6-align-rest-api-client.md | Marks sub-issue tasks as DONE and records clarifying decisions. |
| Cargo.lock | Records the added protocol dependency in the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d typed ApiClient
…oid panics ApiClient methods now call the fallible methods on ApiHttpClient and map failures to ClientError::TransportError, fulfilling the 'never panics' contract. Also made ClientError::ApiError variant fields implicitly public (since the enum itself is pub).
josecelano
force-pushed
the
1944-si-6-align-rest-api-client
branch
from
June 30, 2026 09:55
7acf385 to
58e7ee0
Compare
Clippy requires # Panics documentation on all public methods that may panic. Added the missing doc sections to all 11 ApiHttpClient public methods that use .unwrap().
- Added variant for URL construction failures. - Added so works for transport errors. - Changed all fallible methods to return instead of . - Made fallible (returns ) instead of panicking. - methods now use directly with no calls.
Member
Author
|
ACK bbd2936 |
josecelano
added a commit
that referenced
this pull request
Jul 15, 2026
…ssues/open to docs/issues/closed 174904d chore(ci): point to ADR instead of closed issue spec in deployment-packages.yaml (Jose Celano) 2e2178f docs(skill): document branch-already-exists edge case in cleanup-completed-issues skill (Jose Celano) 1bfd306 chore(issues): archive closed issue specs #1447, #1459, #1505, #1926, #1938, #1944, #1959, #1964, #1965, #1969 to docs/issues/closed (Jose Celano) Pull request description: Archives the following 10 issue specs that are closed on GitHub, moving them from `docs/issues/open/` to `docs/issues/closed/`: | Issue | Title | PR | |-------|-------|-----| | #1447 | Change logging threshold for connection ID error to WARNING | #1975 | | #1459 | Docker Security Overhaul | #1958 | | #1505 | Optimize peer IP list from swarm (implementation rejected) | #1949 | | #1926 | Define package versioning strategy | #1961 | | #1938 | REST API Contract-First Migration EPIC | #1960 (SI-1→5) | | #1944 | SI-6: Align REST API client | #1968 | | #1959 | SI-7: Review tests, align v1 namespace | #1963 | | #1964 | Rename number-of-downloads BTreeMap type alias | #1972 | | #1965 | SI-34: Consolidate duplicate HTTP types | #1974 | | #1969 | SI-8: Eliminate unwraps from REST API client | #1973 | ### Changes Made - All 10 specs verified `CLOSED` on GitHub with merged PRs - Files/directories moved via `git mv` to `docs/issues/closed/` - Frontmatter updated: `status: done`, `spec-path` → closed path, `last-updated-utc: 2026-07-15` - Workflow checkpoints updated and progress log entries added - Supplementary files in multi-file directories updated with new paths - Parent EPIC specs (#1669, #1938) updated: subissue status, spec paths, and table references - Fixed stale references in `.github/workflows/deployment-packages.yaml`, `docs/adrs/`, and `docs/issues/closed/` files - Pre-commit and pre-push hooks passed (linter, tests, doc-tests, nightly checks) ACKs for top commit: josecelano: ACK 174904d Tree-SHA512: c7e38354bb5898fc5125a0b68fd61a02ca273689593638eb3b955b9176038ef82c40fc98b0a2f722eb18d8d429d2f2dbb6f205eee3061211ab9906c5be46942b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1944
Summary
Align the REST API client package (
torrust-tracker-rest-api-client) with therest-api-protocolpackage by introducing a high-level typedApiClientalongside the existing low-levelApiHttpClient(renamed fromClient).Changes
Refactoring
Client→ApiHttpClientacross the entire codebase (client, contract tests, E2E tests)ApiClientstruct with typed methods returningResult<DtoType, ClientError>for all 10 REST API endpointsClientErrorenum with three variants:TransportError,ApiError(with status + body),DeserializationErrorRe-exports
AddKeyFormre-exported fromrest-api-protocolat the top of the moduleApiClientandApiHttpClientre-exported fromv1::mod.rsDependencies
torrust-tracker-rest-api-protocolas a dependency ofrest-api-clientDocumentation
# ErrorssectionsVerification